Skip to content

Add comprehensive unit and integration tests for UserViewController, UserMapper, and UserService#37

Merged
Martin-E-Karlsson merged 10 commits into
mainfrom
user-tests
Apr 21, 2026
Merged

Add comprehensive unit and integration tests for UserViewController, UserMapper, and UserService#37
Martin-E-Karlsson merged 10 commits into
mainfrom
user-tests

Conversation

@Martin-E-Karlsson

@Martin-E-Karlsson Martin-E-Karlsson commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Unit and Integration tests for:

  • UserViewController
  • UserMapper
  • UserService

Changes to the controllers will have to be made if endpoints are rearranged and when further features are added.

Summary by CodeRabbit

  • Tests
    • Expanded test coverage for user management: added comprehensive controller, service (unit & integration), and mapper tests covering signup/login flows, profile view/edit/update, role-based dashboards and lists, authorization checks, CRUD operations, and mapping behaviors.

- First implementation of tests
- Added UserServiceIntegrationTest for integration tests.
- Updated unit tests for Mapper, Service and ViewController to match those in Visa tests.
- Added display name annotations to all user tests.
- Updated User Tests to use UserPrincipal instead of SecurityUSer
@Martin-E-Karlsson Martin-E-Karlsson linked an issue Apr 20, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@Martin-E-Karlsson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 59 minutes and 5 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 5 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c5312c70-b24e-45ae-ac92-acbb54b54bd5

📥 Commits

Reviewing files that changed from the base of the PR and between e9fe0fc and 03aea0c.

📒 Files selected for processing (2)
  • src/test/java/org/example/visacasemanagementsystem/user/controller/UserViewControllerTest.java
  • src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java
📝 Walkthrough

Walkthrough

Adds multiple new test suites (controller, service unit, service integration, mapper) and makes TestcontainersConfiguration public; no production code logic changed.

Changes

Cohort / File(s) Summary
Test Configuration
src/test/java/.../TestcontainersConfiguration.java
Changed class visibility from package-private to public.
Controller Tests
src/test/java/.../user/controller/UserViewControllerTest.java
New Spring MVC @WebMvcTest using MockMvc; tests signup/login views, profile view/edit flows, role-based access, dashboards, and user listing; mocks services and simulates authentication.
Service Layer Tests
src/test/java/.../user/service/UserServiceTest.java, src/test/java/.../user/service/UserServiceIntegrationTest.java
Added unit tests (Mockito) for UserService covering create/update/delete/authorization/error mapping and retrieval; added Spring Boot integration tests with H2 for persistence-backed UserService scenarios and validateProfileAccess checks.
Mapper Tests
src/test/java/.../user/mapper/UserMapperTest.java
Added unit tests for UserMapper covering toDTO, toEntity, updateEntityFromDTO, null-safety, and preservation/non-preservation of sensitive fields (password).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • EdvinSandgren
  • eeebbaandersson

Poem

"Hop hop, I test and I prance, 🐰
Mocking services in a merry dance,
DTOs and profiles checked with care,
Integration H2 gives persistence flair,
Now the codebase hops with confidence rare."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the primary changes: adding comprehensive unit and integration tests for three core components (UserViewController, UserMapper, UserService).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch user-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (4)
src/test/java/org/example/visacasemanagementsystem/user/mapper/UserMapperTest.java (1)

39-44: Consider also asserting dto.id().

toDTO maps user.getId() into UserDTO.id, but the test never asserts it. Setting user.setId(...) in the arrange block and adding an assertThat(dto.id()).isEqualTo(...) would make the mapping contract fully covered and guard against accidental field-order regressions in the UserDTO record.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/user/mapper/UserMapperTest.java`
around lines 39 - 44, The test UserMapperTest does not assert that toDTO
preserves the user ID; update the arrange section to set user.setId(...) with a
known value and add an assertion in the assert block verifying dto.id() equals
that value (e.g., assertThat(dto.id()).isEqualTo(...)); this ensures the toDTO
mapping and the UserDTO.id field are covered and prevents regressions in
UserMapper.toDTO or the UserDTO record.
src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceIntegrationTest.java (2)

224-244: Minor: helpers bypass UserService and persist plaintext passwords.

Persisting "password123" directly to the DB is fine for tests and matches other suites, but note that createUser-path tests (line 35) go through passwordEncoder.encode(...) while update/delete/find tests read rows with plaintext passwords. If any future assertion checks the hashed password, this asymmetry will cause confusing failures. Consider encoding in the helper or routing through userService.createUser.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceIntegrationTest.java`
around lines 224 - 244, The helpers createAndSaveValidUser and createAndSaveUser
persist plaintext "password123" via userRepository, which bypasses UserService
password hashing; update these helpers to either call
userService.createUser(...) so the password is encoded through the normal flow,
or explicitly encode the password with passwordEncoder.encode("password123")
before saving with userRepository.save(...), ensuring tests that inspect stored
passwords see hashed values and stay consistent with createUser-path tests.

110-122: Test is order/seed-dependent.

findAll is asserted with hasSizeGreaterThanOrEqualTo(2). Because @Transactional on @SpringBootTest rolls back per test, this is safe in isolation, but any seed data (e.g. a data.sql or @Sql) will inflate the count and hide regressions where extra rows leak in. A stricter assertion using the two returned emails would be more precise:

Proposed fix
-        createAndSaveValidUser();
-        createAndSaveUser("Admin", UserAuthorization.ADMIN);
+        User u1 = createAndSaveValidUser();
+        User u2 = createAndSaveUser("Admin", UserAuthorization.ADMIN);
@@
-        assertThat(result).hasSizeGreaterThanOrEqualTo(2);
+        assertThat(result)
+                .extracting(UserDTO::email)
+                .contains(u1.getEmail(), u2.getEmail());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceIntegrationTest.java`
around lines 110 - 122, The test findAll_shouldReturnAllUsers relies on a size
check which can be skewed by seeded data; instead capture the two users created
by createAndSaveValidUser() and createAndSaveUser("Admin",
UserAuthorization.ADMIN) and assert that userService.findAll() contains those
users (e.g. by extracting their email fields and asserting the returned list
contains both emails or containsExactlyInAnyOrder those two emails), so change
the assertion to verify the presence of the two specific created users rather
than a minimum list size.
src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java (1)

44-61: Unnecessary stub on userMapper.toEntity for the short-password test.

With MockitoExtension in strict mode, stubbing is fine here because the production code invokes userMapper.toEntity(dto) before the length check. Just flagging that if the service is later refactored to validate the password first (a sensible change), this stub will become a UnnecessaryStubbingException. Not blocking.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java`
around lines 44 - 61, The test stubs userMapper.toEntity(dto) unnecessarily for
createUser_shouldThrowIllegalArgumentException_WhenPasswordIsTooShort; remove
the when(userMapper.toEntity(dto)).thenReturn(new User()) line so the test only
asserts that userService.createUser(dto) throws and still
verifyNoInteractions(userRepository); if you need to keep a stub for other
reasons use Mockito.lenient() on the userMapper.toEntity stub or move that
stubbing into tests that exercise mapping logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@src/test/java/org/example/visacasemanagementsystem/user/mapper/UserMapperTest.java`:
- Around line 39-44: The test UserMapperTest does not assert that toDTO
preserves the user ID; update the arrange section to set user.setId(...) with a
known value and add an assertion in the assert block verifying dto.id() equals
that value (e.g., assertThat(dto.id()).isEqualTo(...)); this ensures the toDTO
mapping and the UserDTO.id field are covered and prevents regressions in
UserMapper.toDTO or the UserDTO record.

In
`@src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceIntegrationTest.java`:
- Around line 224-244: The helpers createAndSaveValidUser and createAndSaveUser
persist plaintext "password123" via userRepository, which bypasses UserService
password hashing; update these helpers to either call
userService.createUser(...) so the password is encoded through the normal flow,
or explicitly encode the password with passwordEncoder.encode("password123")
before saving with userRepository.save(...), ensuring tests that inspect stored
passwords see hashed values and stay consistent with createUser-path tests.
- Around line 110-122: The test findAll_shouldReturnAllUsers relies on a size
check which can be skewed by seeded data; instead capture the two users created
by createAndSaveValidUser() and createAndSaveUser("Admin",
UserAuthorization.ADMIN) and assert that userService.findAll() contains those
users (e.g. by extracting their email fields and asserting the returned list
contains both emails or containsExactlyInAnyOrder those two emails), so change
the assertion to verify the presence of the two specific created users rather
than a minimum list size.

In
`@src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java`:
- Around line 44-61: The test stubs userMapper.toEntity(dto) unnecessarily for
createUser_shouldThrowIllegalArgumentException_WhenPasswordIsTooShort; remove
the when(userMapper.toEntity(dto)).thenReturn(new User()) line so the test only
asserts that userService.createUser(dto) throws and still
verifyNoInteractions(userRepository); if you need to keep a stub for other
reasons use Mockito.lenient() on the userMapper.toEntity stub or move that
stubbing into tests that exercise mapping logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6adccb61-0302-4908-a54c-3c0504123c1c

📥 Commits

Reviewing files that changed from the base of the PR and between 46acde2 and b621d02.

📒 Files selected for processing (5)
  • src/test/java/org/example/visacasemanagementsystem/TestcontainersConfiguration.java
  • src/test/java/org/example/visacasemanagementsystem/user/controller/UserViewControllerTest.java
  • src/test/java/org/example/visacasemanagementsystem/user/mapper/UserMapperTest.java
  • src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceIntegrationTest.java
  • src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java

@Martin-E-Karlsson Martin-E-Karlsson changed the title User tests Add comprehensive unit and integration tests for UserViewController, UserMapper, and UserService Apr 20, 2026
@eeebbaandersson
eeebbaandersson self-requested a review April 20, 2026 14:33

@eeebbaandersson eeebbaandersson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ser fint ut tycker jag!

- Made more tests function with the most recent security update
- Added missing SecurityConfig import in UserViewControllerTest

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java (2)

83-107: Assert the saved entity’s security-sensitive fields.

This test stubs password encoding but never proves the encoded password or forced USER authorization is saved. save(any(User.class)) would also pass if UserService saved the wrong instance.

Strengthen the assertion
         CreateUserDTO dto = new CreateUserDTO(
-                "New User", "new@test.com", "password123", UserAuthorization.USER
+                "New User", "new@test.com", "password123", UserAuthorization.SYSADMIN
         );
@@
         when(userMapper.toEntity(dto)).thenReturn(mappedUser);
         when(passwordEncoder.encode("password123")).thenReturn("encodedPassword");
-        when(userRepository.save(any(User.class))).thenReturn(savedUser);
+        when(userRepository.save(mappedUser)).thenReturn(savedUser);
         when(userMapper.toDTO(savedUser)).thenReturn(expectedDTO);
@@
         // Assert
         assertThat(result).isEqualTo(expectedDTO);
-        verify(userRepository, times(1)).save(any(User.class));
+        assertThat(mappedUser.getPassword()).isEqualTo("encodedPassword");
+        assertThat(mappedUser.getUserAuthorization()).isEqualTo(UserAuthorization.USER);
+        verify(passwordEncoder).encode("password123");
+        verify(userRepository).save(mappedUser);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java`
around lines 83 - 107, The test currently stubs passwordEncoder.encode and
userRepository.save but doesn't assert that UserService.createUser actually
saved a User with the encoded password and forced UserAuthorization.USER; update
the test for createUser to capture/verify the exact entity passed to
userRepository.save (use an ArgumentCaptor or verify with argThat) and assert
that the captured User's password equals the encoded value from
passwordEncoder.encode("password123") and that its authorization equals
UserAuthorization.USER; also ensure userMapper.toEntity(dto) and
userMapper.toDTO(savedUser) interactions remain verified so the flow is
validated.

184-205: Assert the entity role mutation, not only the mocked DTO.

userMapper.toDTO(targetUser) is mocked to return an ADMIN DTO, so this test can pass even if updateUserAuthorization() stops calling targetUser.setUserAuthorization(...).

Strengthen the assertion
         // Assert
+        assertThat(result).isEqualTo(expectedDTO);
+        assertThat(targetUser.getUserAuthorization()).isEqualTo(UserAuthorization.ADMIN);
         assertThat(result.userAuthorization()).isEqualTo(UserAuthorization.ADMIN);
         verify(userRepository).save(targetUser);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java`
around lines 184 - 205, The test currently only asserts the mocked UserDTO so it
can pass without the entity being mutated; after calling
userService.updateUserAuthorization(userId, UserAuthorization.ADMIN) assert the
actual entity mutation by checking targetUser.getUserAuthorization() (or capture
the saved entity via an ArgumentCaptor on userRepository.save and assert its
getUserAuthorization() is ADMIN) in addition to existing DTO assertion;
reference updateUserAuthorization, userService, targetUser, userRepository.save
and userMapper.toDTO when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/test/java/org/example/visacasemanagementsystem/user/controller/UserViewControllerTest.java`:
- Around line 55-136: The tests for public endpoints are incorrectly annotated
with `@WithMockUser`, making them authenticated and masking regressions; remove
the `@WithMockUser` annotation from the test methods
userSignupForm_ShouldReturnSignupView,
createUser_WithValidData_ShouldRedirectToLogin,
createUser_WithDuplicateEmail_ShouldReturnSignupViewWithError,
createUser_WithShortPassword_ShouldReturnSignupViewWithError, and
userLoginForm_ShouldReturnLoginView so they run as anonymous requests (keep
existing csrf() on POST tests and existing mocks/verify calls intact).

---

Nitpick comments:
In
`@src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java`:
- Around line 83-107: The test currently stubs passwordEncoder.encode and
userRepository.save but doesn't assert that UserService.createUser actually
saved a User with the encoded password and forced UserAuthorization.USER; update
the test for createUser to capture/verify the exact entity passed to
userRepository.save (use an ArgumentCaptor or verify with argThat) and assert
that the captured User's password equals the encoded value from
passwordEncoder.encode("password123") and that its authorization equals
UserAuthorization.USER; also ensure userMapper.toEntity(dto) and
userMapper.toDTO(savedUser) interactions remain verified so the flow is
validated.
- Around line 184-205: The test currently only asserts the mocked UserDTO so it
can pass without the entity being mutated; after calling
userService.updateUserAuthorization(userId, UserAuthorization.ADMIN) assert the
actual entity mutation by checking targetUser.getUserAuthorization() (or capture
the saved entity via an ArgumentCaptor on userRepository.save and assert its
getUserAuthorization() is ADMIN) in addition to existing DTO assertion;
reference updateUserAuthorization, userService, targetUser, userRepository.save
and userMapper.toDTO when making the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eb02c29d-a0a6-49d1-bdb0-01eceb228af3

📥 Commits

Reviewing files that changed from the base of the PR and between b621d02 and e9fe0fc.

📒 Files selected for processing (3)
  • src/test/java/org/example/visacasemanagementsystem/user/controller/UserViewControllerTest.java
  • src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceIntegrationTest.java
  • src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/org/example/visacasemanagementsystem/user/service/UserServiceIntegrationTest.java

- Removed @WithMockUser as per CodeRabbit suggestion
- Strengthened test assertion as per CodeRabbit suggestion
- Strengthened test assertion as per CodeRabbit suggestion
- File missed in last push
@Martin-E-Karlsson
Martin-E-Karlsson merged commit e396425 into main Apr 21, 2026
2 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tests for User package

2 participants